Consolidate integration tests for EventCacheStore#6061
Merged
Hywan merged 29 commits intomatrix-org:mainfrom Feb 5, 2026
Merged
Consolidate integration tests for EventCacheStore#6061Hywan merged 29 commits intomatrix-org:mainfrom
EventCacheStore#6061Hywan merged 29 commits intomatrix-org:mainfrom
Conversation
… tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…ion tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…n tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
… tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…ests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…ation tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
… integration tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…on tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…integration tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…egration tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…n tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
The tests removed are either covered by other tests or ensure properties that shouldn't exist - e.g., that new chunks can link to chunks that haven't been put into the store yet. Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
… store Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…rom integration tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…to indexeddb tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6061 +/- ##
==========================================
- Coverage 89.84% 89.82% -0.02%
==========================================
Files 362 362
Lines 100433 99931 -502
Branches 100433 99931 -502
==========================================
- Hits 90230 89760 -470
+ Misses 6678 6668 -10
+ Partials 3525 3503 -22 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
…hunk Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
… integration tests Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
andybalaam
approved these changes
Feb 4, 2026
Member
andybalaam
left a comment
There was a problem hiding this comment.
This looks good to me, but I'd like @Hywan to take a quick look please!
Hywan
approved these changes
Feb 5, 2026
Member
Hywan
left a comment
There was a problem hiding this comment.
Excellent! Thank you for this great work. It was a long-standing issue, and clearly not so pleasant to do. I really appreciate the efforts here!
crates/matrix-sdk-base/src/event_cache/store/integration_tests.rs
Outdated
Show resolved
Hide resolved
Comment on lines
+1608
to
+1613
| if updated < 1 { | ||
| return Err(rusqlite::Error::QueryReturnedNoRows); | ||
| } | ||
| if updated > 1 { | ||
| return Err(rusqlite::Error::QueryReturnedMoreThanOneRow); | ||
| } |
Comment on lines
+1626
to
+1631
| if updated < 1 { | ||
| return Err(rusqlite::Error::QueryReturnedNoRows); | ||
| } | ||
| if updated > 1 { | ||
| return Err(rusqlite::Error::QueryReturnedMoreThanOneRow); | ||
| } |
| /// relevant when calling [`EventCacheStore::handle_linked_chunk_updates`], | ||
| /// which consumes a list of [`Update`]s. When processing this list, if | ||
| /// one of the [`Update`]s fails, the previous updates in the list | ||
| /// will not be reversed. |
| linked_chunk_id: LinkedChunkId<'_>, | ||
| updates: Vec<Update<Item, Gap>>, | ||
| ) { | ||
| ) -> Result<(), RelationalLinkedChunkError> { |
| assert!(chunks.next().is_none()); | ||
| } | ||
|
|
||
| async fn test_linked_chunk_exists_before_referenced(&self) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
There are a number of tests in the SQLite and IndexedDB implementations of the
EventCacheStorewhich are general enough to be moved into theEventCacheStoreIntegrationTeststrait. This pull request consolidates these tests to the extent possible.Changes
Consolidating Tests
For existing test functions in
matrix-sdk-sqlite::event_cache_storeandmatrix-sdk-indexeddb::event_cache_store, one of the following actions were taken.EventCacheStoretrait, so it was converted into a generalized integration test and copied into theEventCacheStoreIntegrationTeststrait.EventCacheStoretrait, so it was moved into theEventCacheStoreIntegrationTeststrait.For the action taken for each existing function, see the tables at the end of this pull request.
Behavioral Inconsistencies
The process of converting existing tests into integration tests revealed that there were some behavioral differences between
MemoryStore's implementation ofEventCacheStoreand other existing implementations - i.e.,SQLiteEventCacheStoreandIndexeddbEventCacheStore.MemoryStoredoes not operate transactionally - i.e., a failure in a list of updates does not rollback previous updates in that list.matrix_sdk_sqliteandmatrix_sdk_indexeddband documenting thatMemoryStoredoes not operate transactionally.MemoryStoredoes not allow referencing a chunk before it exists in the store - e.g.,Update::NewItemsChunk::nextmay not provide a chunk identifier that is not in the store.SQLiteEventCacheStoreandIndexeddbEventCacheStoreso that they too would not allow referencing a chunk before it exists in the store.test_linked_chunk_exists_before_referenced.MemoryStoreallows duplicate chunk identifiers - i.e., one can add a new chunk and assign it a chunk identifier that already exists in the store.MemoryStoreallows duplicateChunkIdentifier's #6095 and will likely be handled in a separate pull request.Further Work
Since starting work on this pull request, some changes have been introduced to the
SQLiteEventCacheStore(see #6065). These changes ensure that theSQLiteEventCacheStorecan store the same event in multiple chunks and, furthermore, add a test to confirm this behavior.These changes have not yet been applied to
IndexeddbEventCacheStore(see #6094), so corresponding tests for these new behaviors could not be turned into integration tests yet.Tables
matrix-sdk-sqlitetest_pool_sizetest_linked_chunk_new_items_chunktest_linked_chunk_new_gap_chunktest_linked_chunk_replace_itemtest_linked_chunk_remove_chunktest_linked_chunk_push_itemstest_linked_chunk_remove_itemtest_linked_chunk_detach_last_itemstest_linked_chunk_start_end_reattach_itemstest_linked_chunk_cleartest_linked_chunk_multiple_roomstest_linked_chunk_update_is_a_transactiontest_filer_duplicate_events_no_eventstest_load_last_chunktest_load_last_chunk_with_a_cycletest_load_previous_chunktest_event_chunks_allows_same_event_in_room_and_threadmatrix-sdk-indexeddbtest_linked_chunk_new_items_chunktest_add_gap_chunk_and_delete_immediatelytest_linked_chunk_new_gap_chunktest_linked_chunk_replace_itemtest_linked_chunk_remove_chunktest_linked_chunk_push_itemstest_linked_chunk_remove_itemtest_linked_chunk_detach_last_itemstest_linked_chunk_start_end_reattach_itemstest_linked_chunk_cleartest_linked_chunk_multiple_roomstest_linked_chunk_update_is_a_transactiontest_load_last_chunktest_load_previous_chunkCloses #5342.
Signed-off-by: Michael Goldenberg m@mgoldenberg.net